Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Shared state pattern (spin-off of #427) #430

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

mxgrey
Copy link
Collaborator

@mxgrey mxgrey commented Nov 20, 2024

This PR is spun out of #427 and focuses entirely on moving the API structs to a "shared state" pattern, reducing the prevalence of Arc in the API without any functional changes.

This one is very subjective so I don't mind discarding it, but I felt that the prevalence of Arc in the API was adding noise that is seldom relevant to the average user. In this PR, I've refactored the API a little so that Node, Subscription, Publisher, Client, and Service each have a respective _State structure which is public and represents the underlying instance. At the same time, Node, Subscription, etc are now type aliases of the form

  • pub type Node = Arc<NodeState>;
  • pub type Subscription<T> = Arc<SubscriptionState<T>>;

This does not lead to any actual change in the structure or memory management of any of these types; it's a purely superficial renaming. But it reduces how often Arc is tossed around in the API and thereby streamlines the 95% (my own estimate) of use cases where Arc is just an implementation detail. At the same time, it does not interfere with the remaining 5% of use cases where the Arc is relevant, e.g. if a user wants to hold onto a Weak<Node> for some reason.

Signed-off-by: Michael X. Grey <greyxmike@gmail.com>
Signed-off-by: Michael X. Grey <greyxmike@gmail.com>
Signed-off-by: Michael X. Grey <greyxmike@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant